Skip to content

PIP-209: Build C++ client in standalone repo#2

Merged
BewareMyPower merged 4 commits intoapache:mainfrom
merlimat:build-standalone-client
Sep 29, 2022
Merged

PIP-209: Build C++ client in standalone repo#2
BewareMyPower merged 4 commits intoapache:mainfrom
merlimat:build-standalone-client

Conversation

@merlimat
Copy link
Contributor

@merlimat merlimat commented Sep 29, 2022

Changs to build C++ client in the new standalone repo

@Demogorgon314
Copy link
Member

Demogorgon314 commented Sep 29, 2022 •
edited
Loading

Look like the build still failed. The ExternalTest.proto file is missing.

Could not make proto path relative: /Users/wangkai/Developer/github/pulsar-client-cpp/tests/proto/ExternalTest.proto: No such file or directory
gmake[2]: *** [tests/CMakeFiles/main.dir/build.make:74: generated/tests/Test.pb.cc] Error 1
gmake[2]: *** Waiting for unfinished jobs....
gmake[1]: *** [CMakeFiles/Makefile2:565: tests/CMakeFiles/main.dir/all] Error 2
gmake[1]: *** Waiting for unfinished jobs....
[ 67%] Building CXX object python/CMakeFiles/_pulsar.dir/src/message.cc.o
[ 68%] Building CXX object python/CMakeFiles/_pulsar.dir/src/authentication.cc.o
[ 68%] Building CXX object python/CMakeFiles/_pulsar.dir/src/reader.cc.o
[ 69%] Building CXX object python/CMakeFiles/_pulsar.dir/src/schema.cc.o
[ 69%] Building CXX object python/CMakeFiles/_pulsar.dir/src/cryptoKeyReader.cc.o
[ 70%] Building CXX object python/CMakeFiles/_pulsar.dir/src/exceptions.cc.o
[ 70%] Building CXX object python/CMakeFiles/_pulsar.dir/src/utils.cc.o
[ 71%] Linking CXX shared library _pulsar.so
ld: warning: -undefined dynamic_lookup may not work with chained fixups
[ 71%] Built target _pulsar
gmake: *** [Makefile:136: all] Error 2

@merlimat merlimat force-pushed the build-standalone-client branch from cb6a81d to 00ac2b2 Compare September 29, 2022 05:03
@merlimat
Copy link
Contributor Author

merlimat commented Sep 29, 2022

Look like the build still failed. The ExternalTest.proto file is missing.

Yes, the build should be fixed now

@Demogorgon314
Copy link
Member

Demogorgon314 commented Sep 29, 2022

In my local env, the Version.h is not generated into include dir, so its builds failed.

[ 69%] Building CXX object tests/CMakeFiles/main.dir/VersionTest.cc.o
/Users/wangkai/Developer/github/pulsar-client-cpp/tests/VersionTest.cc:24:5: error: expected expression
    ASSERT_GE(PULSAR_VERSION, 2000000);
    ^
/usr/local/include/gtest/gtest.h:2096:32: note: expanded from macro 'ASSERT_GE'
# define ASSERT_GE(val1, val2) GTEST_ASSERT_GE(val1, val2)
                               ^
/usr/local/include/gtest/gtest.h:2072:3: note: expanded from macro 'GTEST_ASSERT_GE'
  ASSERT_PRED_FORMAT2(::testing::internal::CmpHelperGE, val1, val2)

@merlimat
Copy link
Contributor Author

merlimat commented Sep 29, 2022

In my local env, the Version.h is not generated into include dir, so its builds failed.

I had one more change that I missed to include in the commit. fixed now.

@BewareMyPower
Copy link
Contributor

BewareMyPower commented Sep 29, 2022

It still doesn't work.

-- Pulsar Client version: 3.0.0-pre
Traceback (most recent call last):
  File "/Users/xuyunze/github.com/BewareMyPower/pulsar-client-cpp/build-support/gen-pulsar-version-macro.py", line 23, in <module>
    version = open("version.txt").read()
FileNotFoundError: [Errno 2] No such file or directory: 'version.txt'
-- Pulsar Client version macro:

Demogorgon314
Demogorgon314 reviewed Sep 29, 2022
View reviewed changes
BewareMyPower
BewareMyPower requested changes Sep 29, 2022
View reviewed changes
Demogorgon314
Demogorgon314 approved these changes Sep 29, 2022
View reviewed changes
Copy link
Member

@Demogorgon314 Demogorgon314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be build successfully now. 🎉

@BewareMyPower BewareMyPower merged commit 12c69f9 into apache:main Sep 29, 2022
@merlimat merlimat deleted the build-standalone-client branch September 29, 2022 05:58
BewareMyPower added a commit that referenced this pull request Sep 13, 2023
@BewareMyPower
[fix] Add the curl wrapper to avoid inconsistent curl options (#313)
787bfd0 
### Motivation

When libcurl is used in `AuthOauth2`, the `CURLOPT_NOSIGNAL` option is
not set, i.e. it will be the default value so that the
`Curl_resolv_timeout` function might crash in multi-threading
environment.

```
#2 0xf630 in _L_unlock_13 from /lib64/libpthread.so.0 (0x34)
#3 0x2e6c7f in Curl_failf from /usr/local/bin/***/libpulsar.so (0x6f)
#4 0x30a285 in Curl_resolv_timeout from /usr/local/bin/***/libpulsar.so (0x95)
```

Since there are many duplicated code when calling curl C APIs, it's hard to
notice that `CURLOPT_NOSIGNAL` is not configured in `AuthOauth2`.

### Modifications

Introduce a `CurlWrapper` class that sets the same options to reduce the
duplicated code and adapting consistent behaviors unless a few options.
BewareMyPower added a commit to streamnative/pulsar-client-cpp that referenced this pull request Sep 14, 2023
@BewareMyPower
[fix] Add the curl wrapper to avoid inconsistent curl options (apache…
57f848f 
…#313)

### Motivation

When libcurl is used in `AuthOauth2`, the `CURLOPT_NOSIGNAL` option is
not set, i.e. it will be the default value so that the
`Curl_resolv_timeout` function might crash in multi-threading
environment.

```
apache#2 0xf630 in _L_unlock_13 from /lib64/libpthread.so.0 (0x34)
apache#3 0x2e6c7f in Curl_failf from /usr/local/bin/***/libpulsar.so (0x6f)
apache#4 0x30a285 in Curl_resolv_timeout from /usr/local/bin/***/libpulsar.so (0x95)
```

Since there are many duplicated code when calling curl C APIs, it's hard to
notice that `CURLOPT_NOSIGNAL` is not configured in `AuthOauth2`.

### Modifications

Introduce a `CurlWrapper` class that sets the same options to reduce the
duplicated code and adapting consistent behaviors unless a few options.

(cherry picked from commit 787bfd0)
@jato-c8y jato-c8y mentioned this pull request Nov 30, 2023
Open
2 tasks
@erobot erobot mentioned this pull request Feb 1, 2024
Merged
5 tasks
@liyinxin liyinxin mentioned this pull request Feb 17, 2025
Open
@haolujun haolujun mentioned this pull request Apr 27, 2025
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BewareMyPower BewareMyPower BewareMyPower approved these changes

@Demogorgon314 Demogorgon314 Demogorgon314 approved these changes

@codelipenghui codelipenghui Awaiting requested review from codelipenghui

@RobertIndie RobertIndie Awaiting requested review from RobertIndie

Assignees

@merlimat merlimat

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@merlimat @Demogorgon314 @BewareMyPower